Skip to content

feat: buffer map results and move to non-blocking#174

Merged
rubensworks merged 3 commits into
comunica:masterfrom
milafrerichs:fix/90-map-display
May 6, 2025
Merged

feat: buffer map results and move to non-blocking#174
rubensworks merged 3 commits into
comunica:masterfrom
milafrerichs:fix/90-map-display

Conversation

@milafrerichs

Copy link
Copy Markdown
Contributor

Description

use requestAnimationFrame to handle the parsing and updating of the map in between renderes of the browser
also use buffering to add as much points as possible

Relaxed Issue

#90

use requestAnimationFrame to handle the parsing and updating of the map
in between renderes of the browser
also use buffering to add as much points as possible

@rubensworks rubensworks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Comment thread src/ldf-client-ui.js Outdated

// If the given result contains geospatial data, show it on the map
_handleGeospatialResult: function (bindings) {
_process_map_data: function () {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_process_map_data: function () {
_processMapData: function () {

Comment thread src/ldf-client-ui.js Outdated
if (!this._mapResultsBuffer) this._mapResultsBuffer = [];
this._mapResultsBuffer.push(bindings);

requestAnimationFrame(this._process_map_data.bind(this));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need a flag here to call requestAnimationFrame only once. After the callback is invoked, the flag can be reset, to allow future requestAnimationFrame calls.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, something like this:

      if (this._mapResultsBuffer.length > 0) {
        if (!this._processingScheduled) {
          this._processingScheduled = true;
          requestAnimationFrame(this._processMapData.bind(this));
        }
      }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

Comment thread src/ldf-client-ui.js Outdated
// Show map if it's not visible yet
if (!self.$mapWrapper.is(':visible'))
self.$mapWrapper.show();
if (this._mapResultsBuffer.length > 0) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we just pushed something to the buffer, this length check can be removed.

@milafrerichs milafrerichs requested a review from rubensworks May 6, 2025 12:19
@rubensworks rubensworks merged commit 3b08336 into comunica:master May 6, 2025
18 checks passed
@rubensworks

Copy link
Copy Markdown
Member

Nice work @milafrerichs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants